Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vehicle autofire #33

Merged
merged 31 commits into from
Feb 5, 2024
Merged

Conversation

Doubleumc
Copy link
Contributor

@Doubleumc Doubleumc commented Oct 29, 2023

About the pull request

Convert vehicle hardpoints from using their bespoke firing system to one structured closely on handheld guns and deployables such as the M2C. Now using the autofire component. Much like handheld weapons it is capable of different firemodes (semi/burst/auto) and changing targets during fire.

Hardpoints were converted to match their old effectiveness as closely as possible; this is intended as a quality of life improvement, not a rebalance. Damage, AP, range, ammo, etc were not touched.

Fire rates were copied over directly. Single-fire weapons with long delays were made semi-auto (e.g. LTB), and those with short delays were made full-auto (e.g. autocannon). Burst-fire weapons with significant extra delays after the burst remained burst-fire (cupola, smokescreen), and the rest were converted to full-auto (e.g. dual cannon). While changing firemodes is easily implemented, no weapon seemed a good candidate for more than one firemode and so that is omitted for now.

Scatter was approximated. The existing accuracy functioned as a percent chance the shot would stray one tile from the target. Gun-style scatter is instead a cone of fire in degrees. No direct conversion is possible, so scatter values are roughly set such that firing at a tile at the edge of the screen should "feel" about as accurate. Closer ranges would experience less spread than before, longer ranges more.

The buffing weapon sensor module was adjusted to work with the new firing system, and effects hardpoint scatter angle and firing rate. Vehicle buffs still use multipliers instead of adding/subtracting as handheld guns do, as a flat +/- adjustment to fire delay would have a significantly different effect on slow firing weapons (e.g. LTB) vs fast firing (e.g. autocannon). One major difference is that burstfire delays are effected and buffs increases the burst density. Before, there was a single cooldown initiated at the start of the burst, and only that cooldown was modified by the buff. Now, since the inter-burst delay is needed by the autofire component both the inter-burst delay and the after-burst delay are modified by buffs.

Activating non-selected hardpoints was removed as not compatible. The issue is that tracking a single click's modifiers is no longer sufficient, it has to track through the whole mousedown-to-mouseup period and the user can change multiple click modifiers in that time. I could not find a method that was satisfactory without a much bigger overhaul of vehicle controls than I'd like to take on in a PR not meant for it. I'm sure it can be done, but that brings up the question of if that's even the control scheme we'd want, in a PR that was never meant to ask that question let alone answer it.

Explain why it's good for the game

Vehicle weapons using gun-like code makes them easier and more familiar to use, and more code commonality makes maintenance just a little bit easier.

Testing Photographs and Procedure

Screenshots & Videos

Put screenshots and videos here with an empty line between the screenshots and the <details> tags.

Changelog

🆑
refactor: vehicle weapons can fire full-auto
del: no more controls for firing vehicle non-selected weapons
/:cl:

Copy link
Member

@fira fira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty cool. Could I convince you to eventually PR it upstream?

code/modules/vehicles/multitile/multitile_interaction.dm Outdated Show resolved Hide resolved
@Doubleumc
Copy link
Contributor Author

I do intend to PR it upstream when its ready. Not quite at feature parity yet, need to wrangle the hardpoint buffs and the controls for not-selected hardpoints.

@Doubleumc Doubleumc marked this pull request as ready for review November 10, 2023 08:39
Copy link
Member

@fira fira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, some notes I can make

code/modules/vehicles/hardpoints/hardpoint.dm Outdated Show resolved Hide resolved
code/modules/vehicles/hardpoints/hardpoint.dm Outdated Show resolved Hide resolved
Comment on lines 673 to 677
if(user)
to_chat(user, SPAN_WARNING("<b>*click*</b>"))
playsound(user, 'sound/weapons/gun_empty.ogg', 25, 1, 5) //5 tile range
else
playsound(src, 'sound/weapons/gun_empty.ogg', 25, 1, 5)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the hardpoint is outside the vehicle, and the user inside, right? why not both?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't think about this, was just directly copied from gun code. Considering internal sounds can be heard externally and vice-versa the difference is minimal, but for consistency's sake with the other hardpoint sounds I'll have the sound played externally.

/// Currently selected target to fire at. Set with set_target().
var/atom/target
/// Current operator (crew) of the hardpoint.
var/mob/hp_operator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unless i'm missing something ideally you'd hook COMSIG_PARENT_QDELETING on the operator and reset_fire if they get deleted to avoid loose references. i know it's a pretty edge case but when it comes to mob references every bit counts

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That case is currently handled indirectly. hp_operator is set by the hardpoint when firing starts, and cleared when firing ends. If an operator were deleted that would cause them to be unbuckled, eventually calling on_unset_interaction(), aborting fire, and the hardpoint would clear hp_operator. Here:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On further thought, since multitile already does the gruntwork for tracking who's in what seat its unnecessary complexity for hardpoint to independently track its user. Removed hp_operator and instead pulls from multitile's seats[] list of crew when necessary.

@Doubleumc Doubleumc requested a review from fira November 12, 2023 21:29
@morrowwolf morrowwolf closed this Jan 15, 2024
@Doubleumc Doubleumc reopened this Jan 25, 2024
@morrowwolf morrowwolf merged commit 0857c77 into cmss13-devs:master Feb 5, 2024
57 checks passed
@Doubleumc Doubleumc deleted the vehicle-autofire branch February 11, 2024 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants